Implement RTL8139 Driver by KieranMusser · Pull Request #81 · UMN-Kernel-Object/ukoos

KieranMusser · 2026-01-12T06:30:58Z

Quite basic and the branch is behind the trunk. I'm making this so people can at least look at the header and comment before anything is final.

Only transmit is implemented, receive is waiting on interrupts and more.

remexre · 2026-01-12T23:16:03Z

src/kernel/arch/riscv64/include.mak

+bgdb: src/kernel/kernel.sym
+	gdb-multiarch src/kernel/kernel.sym \
+		-ex "set substitute-path / $(srcdir)/" \
+		-ex "target remote :1234"
+


#49 should mean you don't need this any more.

True, this is another personal change that I will keep that way in the final version, but I sometimes don't use the tui so it helps to have a shortcut

remexre · 2026-01-12T23:17:06Z

src/kernel/arch/riscv64/paging.c

-/**
- * An Sv39 page table entry.
- */
+


This all changed dramatically in #50; sorry for the breakage.

I'll take this as a good chance to make my code a bit cleaner, which includes merging the new trunk into my branch, so my code will use the new mm then

I'd advise rebase instead of merge, but 👍 in general.

(It probably shouldn't matter if you're just doing it from trunk, but rebase keeps the semantics of "my branch is a series of patches off of some version of trunk" instead of the more annoying truth of "my branch is a dag of repo states"...)

remexre · 2026-01-12T23:29:13Z

src/kernel/drivers/rtl8139.c

+#define READ_U8(addr, off) physical_read_u8(paddr_of_bits(addr + off))
+#define WRITE_U8(addr, off, value) physical_write_u8(paddr_of_bits(addr + off), value)
+
+#define READ_U16(addr, off) physical_read_u16le(paddr_of_bits(addr + off))
+#define WRITE_U16(addr, off, value) physical_write_u16le(paddr_of_bits(addr + off), value)
+
+#define READ_U32(addr, off) physical_read_u32le(paddr_of_bits(addr + off))
+#define WRITE_U32(addr, off, value) physical_write_u32le(paddr_of_bits(addr + off), value)


This is probably a bug against physical.c, but note that these currently to not have READ_ONCE/WRITE_ONCE semantics. The problem comes up in code like this:

WRITE_U32(a, 0, 1); while (READ_U32(a, 4)) ; WRITE_U32(a, 0, 0);

The compiler is perfectly within its rights to optimize this to:

while (READ_U32(a, 4)) ; WRITE_U32(a, 0, 0);

In general, I'm pretty "ehhh" on these helpers -- they should probably use paddr_offset, but also if they're convenient enough that you need them, they're probably convenient enough to add to physical.h.

Probably the right thing here is to make the physical_ functions have those semantics, but I think you probably actually want to:

Discover the device from PCI (see Device model #70).

Allocate a VMA to map the PCI device's registers.

Map the device's registers to that VMA with iomem_map, casting the pointer to a pointer to a struct for the registers.

Use a "real" READ_ONCE/WRITE_ONCE macro to access the fields of that device.

We don't have those real READ_ONCE/WRITE_ONCE macros, so... I should do that.

Correct, these are not meant to be READ/WRITE_ONCE, but rather a simple way for me to do physical reads and writes with u64s (it works on my machine, so I figured I'd deal with READ/WRITE_ONCE later). Is this good? Not really, but once I can map in a physical address, I was planning on having a nice register struct, so instead of
WRITE_REG_32(REG_COM, COM_TE)
we just do something sort of like
rtl->com = COM_TE
(or using WRITE_ONCE ofc, but same idea) which means that these helpers aren't required

Also, I am not sure how this is exactly related to the PCI part, but the PCI "driver" will be rewritten to load data from the device tree so there won't be any hardcoded physical addresses.

👍 For PCI, I just meant reading from/writing to PCI registers

Ah gotcha, then yeah the new PCI driver will also use the same better method as the rtl8139 driver itself, meaning little to no READ/WRITE_U32 or similar

remexre · 2026-01-13T00:08:18Z

src/kernel/drivers/rtl8139.c

+}
+
+
+void rtl8139_test() {


Oh hm, my first instinct would be to say "make this a selftest," but we don't want selftests that depend on the user having certain hardware either...

This probable deserves some thought.

Should this be exposed to userspace, so we can hit it in CI (maybe this would be a build time option, see #62)? Should we accumulate a list of selftests for devices (not drivers) and allow this to be run later in boot?

yeah, I tried a selftest and realized that myself, I had thought of some sort of devices selftest, but I don't want to create too many different types of selftest, so I would be in favor of exposing to userspace and having some userspace testing

remexre · 2026-01-13T00:09:31Z

src/kernel/main.c

  mm_init_virtual(free_va_start, free_va_end);
+  pci_enumerate();
+
+  wfi();


wait for interrupt, I probably shouldn't have committed this but I didn't like the busy loops (my fans turn on)

er yeah, I just meant "why" -- is this just to halt the machine completely?

Exactly, yeah

It's probably worth mentioning that the change to main.c is really just to get the code to do anything interesting at all, but in the future the PCI will be initialized from the device tree, meaning no call to pci_enumerate in main.c

remexre · 2026-01-13T00:10:14Z

src/kernel/pci.c

+paddr mad(u64 addr) {
+  paddr *x;
+  x = (paddr*) (&addr);
+  return *x;
+}


a poor hack that I made for some reason when I wrote PCI - most of PCI will be rewritten, but that will take a bit more time (for now, I wanted the rtl8139 part to be seen by people)

ah, okay; i think the same bullet points for physical memory access as above probably apply to it, though i agree "push something" > "the code is in its final state" (esp. given that the stuff i'm saying to use is just-merged :p )

You're right, and I left a note in #70 about where to include changes to PCI but yeah, this is really just a fix in code that shouldn't really be in trunk anyways

rebase kieran off non-nonsense Signed-off-by: jastintime <130672671+jastintime@users.noreply.github.com>

KieranMusser requested a review from remexre January 12, 2026 06:30

KieranMusser linked an issue Jan 12, 2026 that may be closed by this pull request

RTL8139 driver #73

Open

KieranMusser marked this pull request as draft January 12, 2026 06:34

remexre reviewed Jan 13, 2026

View reviewed changes

remexre mentioned this pull request Jan 13, 2026

Device model #70

Open

jastintime and others added 4 commits February 4, 2026 19:12

rebase non-nonsense off trunk

dcf6172

rebase kieran off non-nonsense Signed-off-by: jastintime <130672671+jastintime@users.noreply.github.com>

sends packet

64495b6

pci: remove macro slop

1f93de1

pci: learn to code

74c2d24

KieranMusser force-pushed the kieran/rtl8139 branch from 43ee3d2 to 74c2d24 Compare February 5, 2026 03:45

		}


		void rtl8139_test() {

Conversation

KieranMusser commented Jan 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants